api: DownOptions.Images becomes a typed ImagePruneMode, validated upfront - #14149
Merged
Merged
Conversation
14 of 39 tasks
docker-agent
left a comment
docker-agent
left a comment
Contributor
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
ndeloof
force-pushed
the
b6-image-prune-mode
branch
from
September 15, 2026 12:58
6167efd to
652b348
Compare
glours
reviewed
Sep 22, 2026
glours
left a comment
glours
left a comment
Contributor
There was a problem hiding this comment.
Something we need to check before we merge IMHO
…ront The legal values of DownOptions.Images lived in pkg/compose (image_pruner.go) while the field itself was a bare string in pkg/api, and the only validation fired inside ImagesToPrune — after down had already removed the project's containers, leaving the teardown half done on a typo. The CLI validated its --rmi flag separately, so only SDK callers were exposed. ImagePruneMode and its three values now live in pkg/api next to the field they constrain (type-aliased in pkg/compose for existing consumers), down() rejects any other value before touching a single resource, and the CLI check reuses the same definition. A unit test runs Down with expectation-free mocks: one daemon call would fail it. Epic docker#14074, B.6. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
ndeloof
force-pushed
the
b6-image-prune-mode
branch
from
October 6, 2026 08:35
652b348 to
31549ed
Compare
ImagesToPrune's own check duplicated the same set of legal ImagePruneMode values down() already validates upfront, risking silent drift if a mode is ever added. Reuse Valid() instead. Suggested-by: glours docker#14149 (comment) Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
down() validates options.Images upfront, but ImagePruner is exported and can be driven directly with a hand-built ImagePruneOptions that bypasses that check. Add a direct test for the Valid() guard added in the previous commit, so the two checks can't silently drift without a test catching it. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
docker-agent
reviewed
Oct 6, 2026
docker-agent
left a comment
docker-agent
left a comment
Contributor
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
glours
pushed a commit
that referenced
this pull request
Oct 6, 2026
ImagesToPrune's own check duplicated the same set of legal ImagePruneMode values down() already validates upfront, risking silent drift if a mode is ever added. Reuse Valid() instead. Suggested-by: glours #14149 (comment) Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Epic #14074, B.6. The legal values of
DownOptions.Imageslived inpkg/composewhile the field was a bare string inpkg/api, and the only validation fired insideImagesToPrune— afterdownhad already removed the project's containers, leaving the teardown half done on a typo (SDK callers only; the CLI validated its--rmiflag separately).ImagePruneModeand its three values move topkg/api, next to the field they constrain (type-aliased inpkg/composefor existing consumers — untyped string literals still compile);down()rejects any other value before touching a single resource;--rmicheck reuses the same definition;Downwith expectation-free mocks: one daemon call would fail it. Mocks verified in sync (make mocks, no drift).🤖 Generated with Claude Code